updated ec spc and weight fn #538

sternhenri · 2019-10-07T21:38:24Z

Updated ec with pseudocode to be moved into Go.

Three open questions:

(@jbenet) happy with weighting function as is (spec. forkFactor) or tune ahead of testnet?
(@Kubuxu) notes on making it easier to implement (sorry)
(@whyrusleeping) (@jbenet) notes on new ticket rule to ensure |tickets(tipset)| == |blocks(tipset)|? Decision time to keep or drop...

Idea is to ensure 1 ticket per block always, with downside being ability for miners to grind through nonces in parallel. Though they would have been able to do that in old way through a batch of k at a time (with k the randomness lookback). I claim the difference is negligible without a working VDF.

src/algorithms/expected_consensus.md

Kubuxu

Some proposed changes:

src/algorithms/expected_consensus.md

anorth · 2019-10-07T23:17:30Z

FYI @ZenGround0

whyrusleeping · 2019-10-08T01:59:20Z

@sternhenri how do i sample randomness from a null block round?

src/algorithms/expected_consensus.md

src/systems/filecoin_blockchain/storage_power_consensus/_index.md

jbenet · 2019-10-08T07:15:48Z

I'm not able to review the weighing function in the next few days -- i'm not paged in enough. Can somebody else? (@nicola @nikkolasg @jzimmerman)

nicola · 2019-10-09T04:48:20Z

Hey team, I am blocked on this being merged since I am doing changes which will require this part to compile and in order to do so, I need to either re-do some things in this PR or wait.

src/systems/filecoin_blockchain/blockchain/block.id

src/algorithms/expected_consensus.md

jbenet · 2019-10-10T01:53:41Z

This needs to be rebased over master. there's a bunch of commits here that are already in master.

sternhenri · 2019-10-11T00:41:48Z

Loads of progress. Still WIP. Left TBD:

Review Block validation
Consensus faults
Power table
Fix randomness retrieval

jbenet · 2019-10-11T02:36:53Z

I think you need to rebase on top of master, i think @nicola made a bunch of changes to get this area to compile

magik6k · 2019-10-12T09:58:04Z

src/algorithms/expected_consensus.md


-The weight should be calculated using big integer arithmetic with order of operations defined above. The multiplication by 1,000 and flooring is meant to help generate uniform weights across implementations.
+We get: 
+- `w[r+1] = w[r] + log2b(totalPowerAtTipset(ts)) * 2^8 + (log2b(totalPowerAtTipset(ts)) * len(ts.blocks) * wRatio_num * 2^8) / (e * wRatio_den)`


log2b(totalPowerAtTipset(ts)) will essentially become a constant at higher network sizes (it may not catch a 2x drop in power), and it's quite painful to compute.

IMO it should be either dropped, or made more 'aggressive' (something between log and linear)

@magik6k it is : log2b(x big.Int) = x.BitLen() - 1
AFAIK, it is there to prevent case where minority miner forks off and slashes other miners.

For which it will work.

You can still slash ~50% of power in the worst case without anyone noticing now

Hi @magik6k, this is super useful feedback.

Remember that this is added to weight at every round, and the other factor is going to be smaller (by wRatio_num/wRatio_den), so while it is indeed a constant, a weight drop by 20% over the space of finality (say ~500 blocks) will be noticeable.

On the flip-side, a power-rental attack will have to be maintained for a bit of time before it becomes irrecoverable. Say I can rent 80% of the network for a few hours, can an honest majority collude to ignore my fork and get the heaviest back in some reasonable amount of time? This is what the more 'aggressive' functions lack...

Thereafter, @Kubuxu is right in terms of what this seeks to prevent as a main reason.

What do you think?

In any case, I need to make clear that log2b is a parameter choice as is wRatio so will update to reflect that. Log2 is simply fave candidate rn.

sternhenri · 2019-10-12T19:33:57Z

Done with main content switches @jbenet, now just need to get it compiling before it can be merged. This branch includes:

Storage power actor changes
all power table accounting
all collateral explanations
consensus faults explanations
randomness sampling throughout the chain
specific code for ticket and election proof generation and validation
new ticket format for @whyrusleeping
weight function specc'd
and more.

Does not include:

storage faults flow
completed block validation flow

sternhenri · 2019-10-13T19:57:06Z

@porcuquine @jbenet any thoughts on test failure here. Work compiles fine for me. Thought I'd ask before diving in in case it's obvious to either of you!

porcuquine · 2019-10-13T20:00:22Z

@sternhenri It looks like this needs make fmt-code changes to be checked in. This would be clearer if we factor the formatting check into a separate CI job. I did the quickest, dirtiest thing for v0.

jbenet · 2019-10-15T11:30:16Z

That's great to hear @sternhenri will review today

jbenet

This LGTM -- looks roughly right, but i'd like @zixuanzh and @nicola to verify it too

jbenet · 2019-10-17T07:20:30Z

src/systems/filecoin_blockchain/storage_power_consensus/_index.md


-The portion of blocks a given miner generates through leader election in EC (and so the block rewards they earn) is proportional to their `Power Fraction` over time. That is, a miner whose storage represents 1% of total storage on the network should mine 1% of blocks on expectation.
+In the case that no miner is eligible to produce a block in a given round of EC, the storage power consensus subsystem will be called by the block producer to attempt another leader election by incrementing the nonce appended to the ticket drawn from the past in order to attempt to craft a new valid `ElectionProof` and trying again.


add note about clock synchrony? (ie why cant i just do all them at once until i win? because people wont accept your blocks early)

jbenet · 2019-10-17T07:24:48Z

src/systems/filecoin_blockchain/storage_power_consensus/_index.md

-At height Y in (39, 67], M1 will attempt to generate an `ElectionProof` using the storage market actor from height 39 for their own power (and an actor from Y for total network power); at height 68, M1 will use the storage market actor from height 67 for their own power, and the storage market actor from height 68 for total power and so on.
+- At height 0, take the genesis block, return its ticket
+- At height n+1, take the heaviest tipset in our chain at height n.
+    - select the block in that tipset with the smallest final ticket, return its ticket


what does "the heaviest tipset in our chain at height n" mean? either:

(a) it means "oh there's exactly one tipset in our chain at heigh n, because "our chain" just means the links walking back block.Parents

(b) or it means "oh there's many tipsets in our chat at height n, because "out chain" means all possible chains we have, with all possible tipsets, we have to pick the heaviest at each epoch.

if (a), just remove the "take the heaviest tipset in our chain at height n", there's only one.
if (b), then i think this is ambiguous and maybe wrong because - what happens if i have tipsetA and tipsetB each the heaviest tipset in epochA and epochB respectively, but NOT part of the same history (dont link to each other).

for any tipset:

select the block in that tipset with the smallest final ticket, return its ticket.

jbenet · 2019-10-17T07:32:41Z

src/systems/filecoin_blockchain/storage_power_consensus/_index.md

+
+#### Picking a Ticket to Seal
+
+When starting to prepare a SEAL in round X, the miner should draw a ticket from X-F with which to compute the SEAL.


why is this here? shouldn't we just say tickets are how we produce randomness in the chain at a given epoch, and then let other things (ie seal) say how they use that randomness?

jbenet · 2019-10-17T07:37:12Z

src/systems/filecoin_blockchain/storage_power_consensus/_index.md

+
+#### Verifying a Seal's ticket
+
+When verifying a SEAL in round Z, a verifier should ensure that the ticket used to generate the SEAL is found in the range of rounds [Z-T-F-G, Z-T-F+G].


this should be defined in Sealing, not here in SPC? unless you want it here because it's relevant to verifiable resource commitment?

jbenet · 2019-10-17T07:40:43Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.id

+    SuspendPower(addr addr.Address, numSectors UVarint)
+    UnsuspendPower(addr addr.Address, numSectors UVarint)
+    RemovePower(addr addr.Address, numSectors UVarint)
+    RemoveAllPower(addr addr.Address, numSectors UVarint)


i dont think we need all these methods, let's only have the ones we absolutely need (smaller potential problem/audit surface)

agree, we only called UpdatePower(powerDelta) in StorageMinerActor for now. powerDelta can be positive or negative which may be a problem. We also don't need addr if we are only changing power for the fromActor who calls the method.

jbenet · 2019-10-17T07:43:53Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.md

+
+The Miner lifecycle in the power table should be roughly as follows:
+- MinerRegistration: A new miner with an associated worker public key and address is registered on the power table by the storage mining subsystem, along with their associated sector size (there is only one per worker).
+- UpdatePower: These power increments and decrements (in multiples of the associated sector size) are called by various storage actor (and must thus be verified by every full node on the network). Specifically:


in multiples of the associated sector size

why not just raw Bytes at every point? or GB?

jbenet · 2019-10-17T07:44:21Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.md

+    - All Power is decremented immediately after a missed PoSt.
+    - Power is decremented immediately after faults are declared, proportional to the faulty sector size.
+    - Power is incremented after a PoSt recovering from a fault.
+    - Power is definitively removed from the Power Table past the sector failure timeout (see {{<sref storage_faults>}})


see the sector state cycle that we're working on in #569

To summarize, only sectors in the Active state will command power. A Sector becomes Active after first PoSt from Committed and Recovering stages. Power is immediately decremented when an Active Sector enters the Failing state (through DeclareFaults or Cron) and when an Active Sector expires.

jbenet · 2019-10-17T07:47:06Z

This needs a rebase unfort-- let's do it soon to avoid much greater divergence.

@zixuanzh wonder how much this conflicts with #569 -- should we merge this first and rebase #569 or vv.?

…on, and EP creation/validation as well as randomness sampling

jbenet · 2019-10-17T10:51:18Z

@sternhenri I rebased for you on top of master (#569 and more) o/

jbenet · 2019-10-17T10:51:59Z

given this is a huge PR, i'm leaning towards merging it before all the comments are addressed (and avoid further merge conflicts

jbenet · 2019-10-17T10:55:26Z

Ok i'm going to merge @sternhenri please address comments as a follow-on PR. Also reconcile w/ new deals/post work -- some things may have been duped/broken

jbenet · 2019-10-17T10:55:57Z

cc @zixuanzh pls review this, and maybe help reconcile w/ new deals work if anything is off

zixuanzh · 2019-10-17T17:42:01Z

src/algorithms/expected_consensus.md


-In order to determine that the mined block was generated by an eligible miner, one must check its `ElectionProof`.
+Note: We draw the miner power from the prior round. This means that if a miner wins a block on their ProvingPeriodEnd even if they have not yet resubmitted a PoSt, they retain their power (until the next round).


not sure if we are saying the same thing, miners keep their power as long as they submit a PoSt when challenged by PoSt Surprise or at the end of a ProvingPeriod.

This is talking specifically about the edge case that is: what if I win a block in the round in which i am challenged (and don't submit). Do I still win or is it forfeited. So this is just meant to avoid fencepost errors. I'll clarify

zixuanzh · 2019-10-17T17:45:50Z

src/algorithms/expected_consensus.md

we have previously defined the slashing mechanism in #383, we might want to check if that's still relevant and if we can use Cron here.

I think this supercedes that.

And yes, I expect the cronActor could call these though in the case of consensus faults it wouldn't be able to (too costly).

zixuanzh · 2019-10-17T17:49:56Z

src/algorithms/expected_consensus.md

+
+It is important to note that there exists a third type of consensus fault directly reported by the `CronActor` on `StorageDeal` failures via the `ReportUncommittedPowerFault` method:
+- (3) `uncommitted power fault` which occurs when a miner fails to submit their `PostProof` and is thus participating in leader election with undue power (see {{<sref storage_faults>}}).


Yes correct, storage faults are declared after a Sector being in the Failing state for three consecutive proving period. A Sector can be put into the Failing state through DeclareFaults or CronAction when a PoSt is missed. Just to be on the same page, pledge collateral will not be slashed for DeclareFaults (miners lose power for declaring faults) but will be slashed when it is spotted by CronAction on the ground of uncommitted power. The amount to be slashed is still TBD cc @jbenet .

Agreed. I expect all of this is defined in the storage faults section.

zixuanzh · 2019-10-17T17:55:19Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.id

+    SuspendPower(addr addr.Address, numSectors UVarint)
+    UnsuspendPower(addr addr.Address, numSectors UVarint)
+    RemovePower(addr addr.Address, numSectors UVarint)
+    RemoveAllPower(addr addr.Address, numSectors UVarint)


agree, we only called UpdatePower(powerDelta) in StorageMinerActor for now. powerDelta can be positive or negative which may be a problem. We also don't need addr if we are only changing power for the fromActor who calls the method.

zixuanzh · 2019-10-17T17:56:29Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.id

+type StorageMiner struct {
+    MinerAddress         addr.Address
+    MinerStoragePower    block.StoragePower
+    MinerSuspendedPower  block.StoragePower


i might be missing something here, any reasons to keep SuspendedPower vs just removing that power?

Two things. I think passing address is better design since we would want to use the same methods for both the cronactor and self-reporting. So I chose to leave it there. Happy to change though.

My thought on suspended power is that it is necessary in order to account for collateral vs power. For instance if I declare a fault and my power drops, I should not be able to withdraw collateral. For that reason we need an intermediary state for power between just "present" and "absent", hence "suspended".

It may be worth our making a state machine for faults @zixuanzh

zixuanzh · 2019-10-17T18:00:04Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_actor.md

+    - All Power is decremented immediately after a missed PoSt.
+    - Power is decremented immediately after faults are declared, proportional to the faulty sector size.
+    - Power is incremented after a PoSt recovering from a fault.
+    - Power is definitively removed from the Power Table past the sector failure timeout (see {{<sref storage_faults>}})


To summarize, only sectors in the Active state will command power. A Sector becomes Active after first PoSt from Committed and Recovering stages. Power is immediately decremented when an Active Sector enters the Failing state (through DeclareFaults or Cron) and when an Active Sector expires.

zixuanzh · 2019-10-17T18:01:15Z

src/systems/filecoin_blockchain/storage_power_consensus/storage_power_consensus_subsystem.id

-    VRFKeyPair    filcrypto.VRFKeyPair
+type ConsensusFaultType union {
+    DoubleForkMiningFault  ConsensusFault
+    ParentGrindingFault    ConsensusFault


should UncommittedPowerFault live here too?

whyrusleeping · 2019-10-21T04:36:40Z

src/systems/filecoin_blockchain/struct/block/block.go


-	return true
+func (bl *Block_I) TicketAtEpoch(epoch ChainEpoch) Ticket {


so does this mean that randomness drawn from a null round is just the randomness of the 'next' block after the series of nulls?

Sorry for lag. No, it means that it is the randomness from the prior "live" round. (null rounds just means nonce increment). I'll specify further

whyrusleeping · 2019-10-21T04:43:45Z

Still unanswered:

@sternhenri how do i sample randomness from a null block round?

sternhenri requested review from jbenet, whyrusleeping and Kubuxu October 7, 2019 21:38

sternhenri mentioned this pull request Oct 7, 2019

Implementation friendly weight calculations #533

Closed

sternhenri commented Oct 7, 2019

View reviewed changes

src/algorithms/expected_consensus.md Show resolved Hide resolved

Kubuxu reviewed Oct 7, 2019

View reviewed changes

jbenet requested changes Oct 8, 2019

View reviewed changes

ZenGround0 mentioned this pull request Oct 8, 2019

Feat/new weight filecoin-project/venus#3532

Merged

2 tasks

nicola reviewed Oct 9, 2019

View reviewed changes

src/systems/filecoin_blockchain/blockchain/block.id Outdated Show resolved Hide resolved

Kubuxu reviewed Oct 9, 2019

View reviewed changes

src/algorithms/expected_consensus.md Outdated Show resolved Hide resolved

sternhenri force-pushed the feat/ec branch from 8e3f346 to a0f3762 Compare October 11, 2019 00:40

sternhenri changed the title ~~updated ec and weight fn~~ updated ec spc and weight fn [WIP] Oct 11, 2019

sternhenri mentioned this pull request Oct 11, 2019

added spec on how you draw SEAL tix #512

Closed

sternhenri force-pushed the feat/ec branch from 6e4cd89 to fb79a8d Compare October 11, 2019 05:36

magik6k reviewed Oct 12, 2019

View reviewed changes

sternhenri force-pushed the feat/ec branch from 752e96d to da1f725 Compare October 12, 2019 19:31

sternhenri mentioned this pull request Oct 13, 2019

EC ticket VRF must use miner address as input #473

Closed

sternhenri changed the title ~~updated ec spc and weight fn [WIP]~~ updated ec spc and weight fn Oct 13, 2019

jbenet approved these changes Oct 17, 2019

View reviewed changes

jbenet and others added 16 commits October 17, 2019 03:21

mining: renamed sector_state fsm

a73b68e

updated ec and weight fn

49364df

nonces insteead of ticket array

06e880e

latest commits

479f2a8

updated ec and weight fn

8f8ed70

nonces insteead of ticket array

a0c4ed1

working slowly through wt fnd

0430204

simplified wt fn

04dbb6e

added special case for log2b

2533377

fix

f23fa7d

fix 2^8 mistake thanks to Kubuxu

e8b52bd

lots of progress on all functions related to ticket creation/validati…

fde3386

…on, and EP creation/validation as well as randomness sampling

fix missed merge conflict

743dcfd

ticket sampling coded.

e1c38ec

added faults, collateral, storage power actor methods

c8b3b06

compiles in spite of heavy commenting out

c57ac59

jbenet force-pushed the feat/ec branch from ad2139f to c57ac59 Compare October 17, 2019 10:50

jbenet merged commit cdf945e into master Oct 17, 2019

jbenet deleted the feat/ec branch October 17, 2019 10:55

zixuanzh reviewed Oct 17, 2019

View reviewed changes

whyrusleeping reviewed Oct 21, 2019

View reviewed changes

sternhenri added a commit that referenced this pull request Nov 19, 2019

address @jbenet's useful feedback i missed from #538

c188350

sternhenri mentioned this pull request Nov 19, 2019

updated randomness sampling in fil to broaden ec solution to all rand… #642

Merged

sternhenri added a commit that referenced this pull request Nov 27, 2019

address @jbenet's useful feedback i missed from #538

c8e84f6


		The portion of blocks a given miner generates through leader election in EC (and so the block rewards they earn) is proportional to their `Power Fraction` over time. That is, a miner whose storage represents 1% of total storage on the network should mine 1% of blocks on expectation.
		In the case that no miner is eligible to produce a block in a given round of EC, the storage power consensus subsystem will be called by the block producer to attempt another leader election by incrementing the nonce appended to the ticket drawn from the past in order to attempt to craft a new valid `ElectionProof` and trying again.


		#### Picking a Ticket to Seal

		When starting to prepare a SEAL in round X, the miner should draw a ticket from X-F with which to compute the SEAL.


		#### Verifying a Seal's ticket

		When verifying a SEAL in round Z, a verifier should ensure that the ticket used to generate the SEAL is found in the range of rounds [Z-T-F-G, Z-T-F+G].


		In order to determine that the mined block was generated by an eligible miner, one must check its `ElectionProof`.
		Note: We draw the miner power from the prior round. This means that if a miner wins a block on their ProvingPeriodEnd even if they have not yet resubmitted a PoSt, they retain their power (until the next round).


		It is important to note that there exists a third type of consensus fault directly reported by the `CronActor` on `StorageDeal` failures via the `ReportUncommittedPowerFault` method:
		- (3) `uncommitted power fault` which occurs when a miner fails to submit their `PostProof` and is thus participating in leader election with undue power (see {{<sref storage_faults>}}).


		return true
		func (bl *Block_I) TicketAtEpoch(epoch ChainEpoch) Ticket {

updated ec spc and weight fn #538

updated ec spc and weight fn #538

Conversation

sternhenri commented Oct 7, 2019

Kubuxu left a comment

Choose a reason for hiding this comment

anorth commented Oct 7, 2019

whyrusleeping commented Oct 8, 2019

jbenet commented Oct 8, 2019

nicola commented Oct 9, 2019

jbenet commented Oct 10, 2019

sternhenri commented Oct 11, 2019 • edited Loading

jbenet commented Oct 11, 2019

Choose a reason for hiding this comment

Kubuxu Oct 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sternhenri commented Oct 12, 2019

sternhenri commented Oct 13, 2019

porcuquine commented Oct 13, 2019

jbenet commented Oct 15, 2019

jbenet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbenet commented Oct 17, 2019

jbenet commented Oct 17, 2019

jbenet commented Oct 17, 2019

jbenet commented Oct 17, 2019

jbenet commented Oct 17, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

whyrusleeping commented Oct 21, 2019

sternhenri commented Oct 11, 2019 •

edited

Loading

Kubuxu Oct 12, 2019 •

edited

Loading